Claude finished @cbini's task in 6m 26s —— View job

PR Review: refactor(dbt): state assessments rebuild

• Gather context and understand PR scope
• Read changed files and analyze code
• Review dbt models and SQL transformations
• Post comprehensive review feedback

This is a well-reasoned rebuild overall. The GROUPING SETS refactor is sound (~85x reduction vs. CUBE is real), the valid_prelim_assessments self-deactivating gate is a clever design, and the self-join fix for rpt_tableau__state_assessments_dashboard_comps is real and meaningful. Issues below are organized by severity.

Critical

1. Broken regexp_extract produces wrong aligned_test_code for grades 3-9

src/dbt/kipptaf/models/pearson/intermediate/int_pearson__student_list_report.sql lines 47-50:

when test_name like '%Mathematics%'
then concat('MAT', regexp_extract(test_name, r'.{6}(.{2})'))
when test_name like '%ELA%'
then concat('ELA', regexp_extract(test_name, r'.{6}(.{2})'))

r'.{6}(.{2})' extracts characters 7-8 of the test name. For "Grade 03 Mathematics" (double-digit), chars 7-8 = "03" → MAT03 ✓. For "Grade 3 Mathematics" (single-digit), chars 7-8 = " M" → MAT M ✗. Grades 3-9 (the majority of state assessment grades) would produce wrong codes. The correct regex is r'Grade (\d+)'.

When aligned_test_code is wrong or NULL (from the missing else), downstream joins to test_code_metadata and valid_prelim_assessments fail silently — rows still count in aggregates but with NULL school_level/grade_range_band/discipline.

Important

2. Dead NJ prelim UNION ALL branch in rpt_tableau__state_assessments_dashboard.sql

Lines 476-591 filter a.results_type = 'Preliminary' in the JOIN. But the assessment_scores CTE (lines 163-234) has its prelim section commented out (/* disabled until next december */), so no rows carry results_type = 'Preliminary'. This branch produces zero rows every run. The current asymmetry — one block commented out, the other live — is the actual bug. Either comment out the UNION ALL branch with the same annotation, or re-enable both together.

3. int_pearson__all_assessments contract enforcement disabled with no tracking

The PR body acknowledges this, but the YML contains no TODO, issue link, or comment. The model is a direct parent of extracts/ models (which have enforced contracts). At minimum, a # TODO: re-enable contract enforcement — see #<issue> comment should be added to the properties file before merge.

4. Race/ethnicity source asymmetry across union branches

int_tableau__state_assessments_demographic_comps.sql:

• NJ official: uses a.aligned_aggregate_ethnicity — race from Pearson assessment file
• NJ prelim: uses inline CASE on e.race_ethnicity — race from PowerSchool enrollment
• FL official: same inline CASE on e.race_ethnicity — race from enrollment

The inline CASE in prelim/FL also lacks an else clause, so any unrecognized race_ethnicity code returns NULL instead of a labeled bucket. Cross-year comparisons for a student can land them in different demographic buckets depending on which branch processes them.

5. valid_prelim_assessments gate relies on unvalidated string match

Lines 42-52: pa.test_type = p.assessment_name must match exactly for the gate to work (e.g., Dagster partition key 'NJSLA' must equal the hardcoded assessment_name = 'NJSLA' in official records). If the naming diverges, prelim data silently flows through even when official scores exist. Needs either an accepted_values test on test_type in the staging model or a comment documenting the coupling.

6. YML: Missing description: on model and all columns — three files

Per src/dbt/CLAUDE.md: "All new or modified models require description: on the model and every column":

• int_tableau__state_assessments_demographic_comps.yml — no model description, no column descriptions
• rpt_tableau__state_assessments_dashboard_comps.yml — check for missing descriptions on modified columns
• The intermediate YML is missing descriptions entirely

7. YML: store_failures: true on individual tests — two files

Per convention: "Do not add store_failures: true to individual tests — the project default handles it."

• int_tableau__state_assessments_demographic_comps.yml line 47
• rpt_tableau__state_assessments_dashboard_comps.yml — same pattern

Both instances should be removed.

Minor

8. ALG01 HS aggregation sums percentages instead of weighted average (pre-existing, now load-bearing)

stg_google_sheets__state_test_comparison_demographics.sql lines 41-43:

sum(percent_proficient) as percent_proficient,
sum(total_students) as total_students,

This sums two percentage values (HS_09 + HS_10) rather than computing a weighted average. The outer SELECT then multiplies percent_proficient * total_students to get total_proficient_students — doubly wrong. Pre-existing, but now directly consumed by the refactored comps pipeline. Should be captured in a tracked issue.

9. Region self-join region_matched = true — undocumented tautology

rpt_tableau__state_assessments_dashboard_comps.sql lines 137-149: When a.comparison_entity = 'Region', the self-join b.comparison_entity = 'Region' matches the same row, so region_matched = true always. Likely intentional but needs a comment to prevent future confusion.

Summary

Issues 1, 2, and 3 should be resolved before merge. Issues 4-7 should either be fixed or tracked in a follow-up issue with a reference added to the code.

@claude - update your review

Claude finished @GabyRangelB's task in 2m 54s —— View job

PR Review: refactor(dbt): state assessments rebuild

• Gather context and understand PR scope
• Read changed files and check prior review issues
• Post updated review feedback

Updated review — all 9 prior issues re-checked against current branch.

Resolved since last review

1. Broken regexp_extract for grades 3-9 — Fixed ✓
int_pearson__student_list_report.sql lines 46–55 now uses r'Grade (\d+)' with lpad(..., 2, '0'). Single-digit grades (3–9) now produce correct codes (MAT03, ELA09, etc.).

2. Dead UNION ALL branch in dashboard — Fixed ✓
The commented-out prelim section in assessment_scores has been removed. Both the prelim UNION ALL in the CTE and the inner join valid_prelim_assessments gate in the NJ prelim SELECT are now live and consistent.

3. Contract enforcement disabled with no tracking — Moot ✓
int_pearson__all_assessments is an intermediate model. Per src/dbt/CLAUDE.md, only rpt_, dim_*, fct_*, and staging models require contract: enforced: true. No tracking needed.

7. store_failures: true on individual tests — Fixed ✓
Removed from both int_tableau__state_assessments_demographic_comps.yml and rpt_tableau__state_assessments_dashboard_comps.yml.

Still open

4. Race/ethnicity source asymmetry across union branches — Still present

int_tableau__state_assessments_demographic_comps.sql:

• NJ official (lines 82–83): reads a.aligned_aggregate_ethnicity — from the Pearson assessment file
• NJ prelim (lines 122–138) and FL official (lines 190–207): inline CASE on e.race_ethnicity — from PowerSchool enrollment

The inline CASE handles null → 'Blank' but has no else clause — any non-null, unrecognized code returns NULL, not 'Blank'. Cross-year comparisons can put the same student in different demographic buckets depending on which branch processes them.

5. valid_prelim_assessments gate relies on unvalidated string match — Still present

Both rpt_tableau__state_assessments_dashboard.sql (line 34) and int_tableau__state_assessments_demographic_comps.sql (line 50):

and pa.test_type = p.assessment_name

If the Dagster test_type partition key diverges from the assessment_name string in official records (e.g. 'NJSLA' vs 'NJSLA-S'), prelim data silently flows through even when official scores exist. Needs either an accepted_values test on test_type in the staging model or a comment documenting the coupling.

6. Missing description: on model and all columns — two YML files — Partially fixed; two files remain

Per src/dbt/CLAUDE.md: "All new or modified models require description: on the model and every column."

• int_pearson__student_list_report.yml — now has full descriptions ✓
• int_tableau__state_assessments_demographic_comps.yml — no model description, no column descriptions ✗
• rpt_tableau__state_assessments_dashboard_comps.yml — no model description, no column descriptions ✗

Minor (pre-existing, tracked)

8. ALG01 HS aggregation sums percentages instead of weighted average — Still present

stg_google_sheets__state_test_comparison_demographics.sql lines 55–56:

sum(percent_proficient) as percent_proficient,
sum(total_students) as total_students,

percent_proficient is the sum of HS_09 + HS_10 percentages, not a weighted average. The outer SELECT at line 86 then multiplies percent_proficient * total_students to compute total_proficient_students, compounding the error. Pre-existing, but now load-bearing. Should be tracked in an issue if not already.

9. Region self-join region_matched = true — undocumented tautology — Still present

rpt_tableau__state_assessments_dashboard_comps.sql lines 137–149: When a.comparison_entity = 'Region', b (filtered to comparison_entity = 'Region') matches the same row, so region_matched is always true for Region rows. Likely intentional but should have a comment to prevent future confusion.

Summary

Issues 4–6 should be resolved or tracked before merge. Issues 8–9 are minor and pre-existing.

Downstream model changes — notice to model owners

The new columns added to int_pearson__all_assessments and int_fldoe__all_assessments made redundant several inline transformation expressions in downstream models. Those models were updated in this PR to remove the duplication. If you own one of the models below, please review the change summary.

Each change was validated by comparing output row counts and metric values against production BigQuery before and after — zero data differences in all cases except where noted.

@awalters — Anthony Walters

rpt_tableau__academic_goals_rollup

What changed: Three branches of state_test_union were computing a subject remap inline. All three now use the pre-computed illuminate_subject column:

Validation: Math/Reading row counts unchanged. 16-row total diff predates this change (FL/iReady/Star upstream data).

Next steps: is_proficient_int, is_approaching_int, is_below_int already read directly from upstream — no further action needed.

rpt_tableau__miami_fast

What changed: is_proficient was a FLOAT64 computed inline. Now reads is_proficient_int (INT64) directly; contract YAML updated to match:

Validation: Zero diff at 39,427 rows. Tableau treats INT64 and FLOAT64 identically for numeric fields.

Next steps: The remaining if(... and fl.is_proficient ...) boolean checks in the disparity/growth CTEs cannot be replaced with is_proficient_int directly, but fast_aggregated_proficiency, sublevel_number, scale_for_proficiency, and points_to_proficiency are now all pre-computed on int_fldoe__all_assessments and may simplify those CTEs further.

rpt_gsheets__kippmiami_payout_roster (currently disabled)

What changed: Two aggregation expressions recomputed is_proficient_int inline:

Validation: Pre-change query confirmed equivalence for all groups — zero mismatches.

Next steps when re-enabling: fast_aggregated_proficiency, sublevel_number, scale_for_proficiency, points_to_proficiency, scale_for_growth, and points_to_growth are now pre-computed on int_fldoe__all_assessments and may simplify the growth and disparity CTEs.

@cbini — Charlie Bini

int_topline__state_assessments_weekly

What changed: Both branches computed is_proficient_int inline. Both now read the column directly:

Validation: Proficient / not-proficient / total counts by region (Camden, Miami, Newark) — zero diff.

dim_assessments

What changed: The state_nj CTE re-derived subject area and test code inline:

Validation: Zero diff across all 24 subject/module combinations.

fct_assessment_scores_enrollment_scoped

What changed: The state_nj CTE computed subject alignment, test code normalization, and period mapping inline:

Validation: Zero diff across all 23 subject/module combinations. Surrogate key stability confirmed (administration_window values unchanged).

Next steps: The state_fl CTE could be similarly simplified — int_fldoe__all_assessments now exposes is_proficient_int, is_approaching_int, is_below_int, and illuminate_subject directly.

@claude please update your review

Claude finished @GabyRangelB's task in 1m 51s —— View job

PR Review: refactor(dbt): state assessments rebuild

• Gather context and understand PR scope
• Read changed files and check prior review issues
• Post updated review feedback

Updated review — all 9 prior issues re-checked against current branch.

Resolved since last review

4. Race/ethnicity source asymmetry — ml_status leg — Partially fixed ✓
The NJ prelim branch now reads e.ml_status directly (line 119) rather than recomputing it inline, closing the aligned_ml_status vs inline-CASE asymmetry for that field. The aggregate_ethnicity asymmetry and the missing else described below remain.

6. Missing model description — int_tableau__state_assessments_demographic_comps.yml — Fixed ✓
Model description and all column descriptions are now present. Only one file remains.

Still open

4. Race/ethnicity source asymmetry and missing else clause — Still present

int_tableau__state_assessments_demographic_comps.sql:

• NJ official (line 82): reads a.aligned_aggregate_ethnicity — from the Pearson assessment file
• NJ prelim (lines 122–139) and FL official (lines 190–207): inline CASE on e.race_ethnicity — from PowerSchool enrollment

Both inline CASE blocks end with when e.race_ethnicity is null then 'Blank' but have no else clause — any non-null, unrecognized code returns NULL instead of 'Blank'. Cross-year comparisons can land the same student in different demographic buckets depending on which branch processes them.

Fix this →

5. valid_prelim_assessments gate relies on unvalidated string match — Still present

Both rpt_tableau__state_assessments_dashboard.sql (line 34) and int_tableau__state_assessments_demographic_comps.sql (line 50):

and pa.test_type = p.assessment_name

If the Dagster test_type partition key diverges from the assessment_name string in official records, prelim data silently flows through even when official scores exist. Needs either an accepted_values test on test_type in the staging model or a comment documenting the expected coupling.

6. Missing model and column descriptions — rpt_tableau__state_assessments_dashboard_comps.yml — Still open

Per src/dbt/CLAUDE.md: "All new or modified models require description: on the model and every column." The file has only data_type: entries — no description: on the model or any of its 17 columns.

Fix this →

Minor (pre-existing, tracked)

8. ALG01 HS aggregation sums percentages instead of weighted average — Still present

stg_google_sheets__state_test_comparison_demographics.sql line 57:

sum(percent_proficient) as percent_proficient,

This sums HS_09 + HS_10 percentages rather than computing a weighted average. The outer SELECT at line 89 then multiplies percent_proficient * total_students to compute total_proficient_students, compounding the error. Pre-existing and now load-bearing. Should be tracked in an issue if not already.

9. Region self-join region_matched = true — undocumented tautology — Still present

rpt_tableau__state_assessments_dashboard_comps.sql lines 132–151: When a.comparison_entity = 'Region', the join b.comparison_entity = 'Region' matches the same row, so region_matched is always true for Region rows. Likely intentional but should have a comment to prevent future confusion.

Summary

Issues 4 (missing else), 5, and 6 should be resolved or tracked before merge.

@claude please update your review

Claude finished @GabyRangelB's task in 2m 23s —— View job

PR Review: refactor(dbt): state assessments rebuild

• Gather context and understand PR scope
• Read changed files and check prior review issues
• Post updated review feedback

Updated review — all prior issues re-checked against current branch.

Resolved since last review

6. Missing model and column descriptions — rpt_tableau__state_assessments_dashboard_comps.yml — Fixed ✓

Full model description and all 16 column descriptions are now present. All three YML files from the original review now have complete descriptions.

Still open

4. Race/ethnicity source asymmetry — missing else clause — Partially fixed

The accepted_values warn test on comparison_demographic_subgroup in int_tableau__state_assessments_demographic_comps.yml (lines 69–83) is a good improvement — it will alert when an unexpected race code produces NULL rather than 'Blank'. However, the underlying CASE blocks in the SQL still lack an else clause, so any non-null, unrecognized race_ethnicity code still silently produces NULL in both the NJ prelim branch (lines 122–139) and FL official branch (lines 190–207). The accepted_values test will warn, but NULL rows will still appear in aggregates until someone investigates. Adding else 'Blank' closes this gap permanently.

Fix this →

5. valid_prelim_assessments gate coupling comment — Partially fixed

int_tableau__state_assessments_demographic_comps.sql now has the comment at line 50 (-- test_type in prelim data matches assessment_name in official records) ✓. However, rpt_tableau__state_assessments_dashboard.sql line 34 has the identical pa.test_type = p.assessment_name join condition with no comment. For consistency and to prevent the same debugging confusion in the other model, that line should also carry the coupling comment.

Fix this →

Minor (pre-existing, tracked)

8. ALG01 HS aggregation sums percentages instead of weighted average — Still present

stg_google_sheets__state_test_comparison_demographics.sql line 57:

sum(percent_proficient) as percent_proficient,

This sums HS_09 + HS_10 percentages rather than a weighted average. The outer SELECT at line 89 then multiplies percent_proficient * total_students to compute total_proficient_students, compounding the error. Pre-existing and load-bearing; should be tracked in an issue if not already.

9. Region self-join region_matched = true — undocumented tautology — Still present

rpt_tableau__state_assessments_dashboard_comps.sql lines 139–151: When a.comparison_entity = 'Region', the join on b.comparison_entity = 'Region' matches the same row, making region_matched always true for Region rows. Likely intentional but a brief comment would prevent future confusion.

Summary

Issues 4 and 5 are the only open blockers. Both are one-line fixes. Issues 8–9 are minor and pre-existing.
·

Issue 5 fixed. On issue 4: we intentionally left out else 'Blank' — the accepted_values warn test is the desired behavior. If a new race_ethnicity code appears that isn't in the CASE, we want to know about it so we can explicitly add it to the mapping. Silently falling through to 'Blank' would hide the new code and we'd never know to handle it.